HDDS-14356. Support OM Service Framework#10346
Conversation
Generated-by: Codex (GPT-5.5)
peterxcli
left a comment
peterxcli
left a comment
There was a problem hiding this comment.
LGTM +1, I thought we need this few days ago when I reviewing the event notification design: #8871 (comment)
|
@peterxcli Good to know that there are more use cases. The background is that @xichen01 requested to add this to support lifecycle service. We already supported this internally for our own use cases. |
|
Thanks @ivandika3 for the info.
Curious is the main use case to have lifecycle service get notified of leader changes? IMO all background services in OM should eventually migrate to derive from this framework. 
|
I'm not entirely sure. I was told there was some discussion about lifecycle service that requires some OM service requirement.
Yes, technically a single background service that only runs on OM leader is ideal. However, IIRC there was some tickets about running background service in follower. However, I think the background service consistency correctness need to be revisited in the future. |
| */ | ||
| public synchronized void start() { | ||
| for (OMService service : services) { | ||
| LOG.debug("Stopping service:{}.", service.getServiceName()); |
| if (omRatisSnapshotProvider != null) { | ||
| omRatisSnapshotProvider.close(); | ||
| } | ||
| serviceManager.stop(); |
There was a problem hiding this comment.
Not an issue, but it's interesting to found that both ServiceManager.starts are implemented but never called.
There was a problem hiding this comment.
Yes, we can make both ServiceManagers to call start if needed in the future. Currently, each SCMService#start is called manually.
|
Thanks @ivandika3 for working on this. Overall looks good to me. |
|
Thanks @peterxcli @ChenSammi for the reviews. |
3 participants
What changes were proposed in this pull request?
Similar to SCM service framework, support OM service framework that can be notified of leader changed. OM stateful service framework can be implemented when needed in HDDS-14790
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14356
How was this patch tested?
UT (https://github.com/ivandika3/ozone/actions/runs/26270725846)